You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Fixed an issue when closing a Modal triggers undesired scrolling in KeyboardAwareScrollView.
💡 Motivation and Context
The issue is because when Modal gets closed we dispatch only onMove/onEnd events. In KeyboardAwareScrollView in onStart method we have a lot of logic that computes current input position before keyboard animation, calculates offset etc. If we skip this stage, then onMove/onEnd may use values from previous keyboard interaction (so if keyboard was closed before and we scrolled for ~100px) we may scroll again, because values from onStart were not updated.
Technically that problem could be fixed in KeyboardAwareScrollView with additional conditions, but we use useKeyboardHandler hook in so many places/components, so if we would use this approach we potentially might need to replicate this fix in KeyboardAvoidingView and other components (and lib users would also need to do that if they use this hook in their projects).
So I think the correct fix is to fix inconsistency in native code.
Funny enough, but the new syncKeyboardPosition now becomes very similar to onKeyboardResized, especially since both functions are used in onApplyWindowInsets and I think that in the future these methods can be unified. Moreover the condition with detection keyboard resize or double keyboard position synchronization also can be unified and as a result we can make the codebase simpler. But this is the next step. First of all we need to merge/release this PR and keep an eye on possible bug reports about new bugs that caused by these changes 👀
Misplaced Call to keepShadowNodesInSync Why: The call to context.keepShadowNodesInSync(eventPropagationView.id) in the onEnd method is incorrectly placed outside the conditional check for interactive keyboards. This could lead to unintended synchronization of shadow nodes, causing potential UI glitches or incorrect state management. Fix: Move this line inside the appropriate conditional block (e.g., when handling non-interactive keyboards) to ensure it only executes under the correct conditions.
Incorrect Event Dispatching in syncKeyboardPosition Why: In the syncKeyboardPosition method, dispatching both Move and End events might lead to redundant or conflicting state updates in React Native's animated system, causing unexpected transitions or performance issues. Fix: Review and adjust the event dispatching logic to ensure it only sends necessary events based on the keyboard's visibility state.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
kirillzyusko
added
🐛 bug
📜 Description
Fixed an issue when closing a
Modaltriggers undesired scrolling inKeyboardAwareScrollView.💡 Motivation and Context
The issue is because when
Modalgets closed we dispatch onlyonMove/onEndevents. InKeyboardAwareScrollViewinonStartmethod we have a lot of logic that computes current input position before keyboard animation, calculates offset etc. If we skip this stage, thenonMove/onEndmay use values from previous keyboard interaction (so if keyboard was closed before and we scrolled for ~100px) we may scroll again, because values fromonStartwere not updated.Technically that problem could be fixed in
KeyboardAwareScrollViewwith additional conditions, but we useuseKeyboardHandlerhook in so many places/components, so if we would use this approach we potentially might need to replicate this fix inKeyboardAvoidingViewand other components (and lib users would also need to do that if they use this hook in their projects).So I think the correct fix is to fix inconsistency in native code.
Funny enough, but the new
syncKeyboardPositionnow becomes very similar toonKeyboardResized, especially since both functions are used inonApplyWindowInsetsand I think that in the future these methods can be unified. Moreover the condition with detection keyboard resize or double keyboard position synchronization also can be unified and as a result we can make the codebase simpler. But this is the next step. First of all we need to merge/release this PR and keep an eye on possible bug reports about new bugs that caused by these changes 👀Closes #1501 #1502
📢 Changelog
Android
onStartevent too fromsyncKeyboardPositionfunction;🤔 How Has This Been Tested?
Tested manually on Pixel 7 Pro (API 36, real device).
📸 Screenshots (if appropriate):
telegram-cloud-document-2-5301142820508179758.mp4
telegram-cloud-document-2-5301142820508179759.mp4
📝 Checklist